fix: derive call-workflow job permissions from caller, not worker (#40169)#40175
Conversation
…0169) buildCallWorkflowJobs previously reverse-engineered the call-<worker> job's permissions block from the union of the worker's job-level permissions. When a worker job declared read-all, Merge() expanded it across GetAllPermissionScopes(), materialising vulnerability-alerts: read into the caller's lockfile. GitHub Actions rejects that scope on GITHUB_TOKEN, causing startup_failure on every run. The call-<worker> job now derives its permissions block from the caller's own declared permissions. extractCallWorkflowPermissions becomes a validation-only step: findUncoveredWorkerPermissions compares the caller's declared permissions against the worker's requirements and emits a compiler warning when the caller is insufficient, without ever modifying the compiled permissions. - pkg/workflow/compiler_safe_output_jobs.go: use caller's permissions, validate worker coverage, warn on gaps - pkg/workflow/call_workflow_permissions.go: add permissionLevelRank and findUncoveredWorkerPermissions helpers; document validation-only role - pkg/cli/workflows/test-copilot-call-workflow.md: widen caller perms to cover its worker under the new model - regenerated affected lock files
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
…workflow-caller-permissions-40169
There was a problem hiding this comment.
⚠️ Not ready to approve
The new worker-coverage validator misses the valid copilot-requests permission scope, which can prevent warnings for real permission gaps and lead to runtime failures.
Pull request overview
This PR fixes a safe-outputs call-workflow compilation design flaw by ensuring the generated call-<worker> job permissions are derived from the caller workflow’s declared permissions, rather than being inflated from the worker’s job permissions. It also adds validation that compares caller vs worker requirements and emits warnings when the caller’s permissions may be insufficient—without mutating the compiled permissions.
Changes:
- Derive
call-<worker>jobpermissions:from the caller’s declared permissions, and validate worker coverage via warnings rather than propagating worker permissions. - Add permission-level comparison helpers and a new unit test for uncovered worker permissions.
- Update the test workflow’s declared permissions and regenerate affected lock files to reflect the caller-derived model.
File summaries
| File | Description |
|---|---|
| pkg/workflow/compiler_safe_output_jobs.go | Switches call-workflow job permissions to be derived from caller permissions and adds warning-only validation against worker permissions. |
| pkg/workflow/call_workflow_permissions.go | Adds permission ranking and uncovered-scope detection helper for validation-only comparison. |
| pkg/workflow/call_workflow_permissions_test.go | Updates existing tests to assert caller-derived permissions and adds coverage for uncovered-scope detection. |
| pkg/cli/workflows/test-copilot-call-workflow.md | Expands declared permissions so the caller covers its worker under the new model. |
| .github/workflows/smoke-call-workflow.lock.yml | Regenerated to reflect caller-derived call-job permissions. |
| .github/workflows/dependabot-campaign.lock.yml | Regenerated to reflect caller-derived call-job permissions. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 1
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The stored frontmatter_hash/body_hash in the lock file no longer matched the values computed by the current hash logic, causing TestHashConsistencyAcrossLockFiles to fail. Recompiling regenerates the hash metadata; the generated workflow body is byte-identical (only the gh-aw-metadata comment changes).
|
|
``
|
|
@copilot review all comments and address unresolved review feedback.
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot review all comments and address unresolved review feedback. Please land the remaining fix and summarize any blockers.
|
|
Please fix the copilot-requests warning and rerun checks.
|
|
Please fix the copilot-requests warning and rerun checks.
|
|
``
|
|
``
|
|
@copilot fix the CI errors |
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot summarize the remaining CI failures and propose the smallest fix.
|
…ions Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Fixed in 3a7cebc. The CI failure was a build error in |
|
@copilot review all comments and address unresolved review feedback.
|
|
Please re-review now that the CI fix is pushed and summarize any remaining blockers.
|
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot please request a fresh review so maintainers can re-evaluate the branch.
|
…github.com/github/gh-aw into fix/call-workflow-caller-permissions-40169
|
``
|
|
``
|
|
`` @copilot review all comments and address the unresolved review feedback on this PR.
|
|
Please address the Copilot review note about the missing
|
Summary
Fixes a bug where
buildCallWorkflowJobswas deriving the permission envelope of a generated call-workflow job from the worker's resolved permissions rather than the caller's declared permissions. This could cause generated workflows to run with an inflated or incorrect permission set that did not match what the caller author intended.Problem
When a
call-workflowjob is compiled, the compiler previously setcallJob.Permissionsto the union of the worker's job-level permissions (rendered). The worker's permissions are derived dynamically and represent what the worker needs, not what the caller has explicitly declared. This violated the principle that a caller should never be granted permissions it did not ask for.Changes
pkg/workflow/compiler_safe_output_jobs.go(high impact)callJob.Permissions = renderedassignment that propagated worker permissions onto the generated job.formatCompilerMessage+os.Stderr+c.IncrementWarningCount()when the caller's declared permissions do not fully cover the scopes the worker requires.Permissionsfield.pkg/workflow/call_workflow_permissions.go(high impact)permissionLevelRankhelper to compare permission levels by severity order.findUncoveredWorkerPermissionshelper that returns the set of worker-required scopes not covered (at sufficient level) by the caller's declared permissions.worker.Get(scope)call insidefindUncoveredWorkerPermissions(commite34e404c1).extractCallWorkflowPermissionsdoc comment: the function is used only for validation, never to inflate caller permissions.pkg/workflow/call_workflow_permissions_test.go(medium impact)TestFindUncoveredWorkerPermissionswith six sub-cases covering:readvswrite)pkg/cli/workflows/test-copilot-call-workflow.md(low impact)issues: readandpull-requests: readto the test workflow's top-levelpermissionsblock to align the fixture with the new validation logic.Behaviour Change
contents: read; worker needscontents: writecontents: writecontents: read; compiler emits a warning thatcontents: writeis uncoveredRisk & Compatibility